Fix #116, 117+118 - Multiple cleanups related to array indexing and range checking #127
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jphickey
force-pushed
the
fix-116-117-118
branch
from
c705a57
to
089a61b
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
CodeQL-coding-standard found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.
jphickey
changed the title
jphickey
force-pushed
the
fix-116-117-118
branch
from
af301da
to
f108c63
Compare
Introduces header files and dedicated types for the purpose of indexing and identification of SC objects and structures. This include ATS/RTS identification, time sequence numbers, word offsets, and command numbers. Use a separate typedef for each, and document its purpose/intent in the description, then update all FSW to use the typedef rather than a simple uint16. Note - in places where the CMD/TLM interface were _not_ using a uint16, the old type is kept. This maintains backward compatibility of the I/F.
This changes all "info" table access to go through an object pointer that is obtained via an accessor method.
Adds inline functions to do range checking, and uses them in all places where the same logic had been copied around. This reduces repetition of logic. Introduces proper data types and wrapper functions to deal with the different types of IDs and indices.
Adds additional range checks to avoid out-of-bounds array access
jphickey
force-pushed
the
fix-116-117-118
branch
from
f108c63
to
ee21c30
Compare
|
NOTE: had to add a fourth commit here (for #121), in order to resolve a (real/preexisting) bug that is now exposed due to the type cleanup. |
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist (Please check before submitting)
Describe the contribution
These commits depend on eachother, so are combined into one PR to avoid conflicts related to splitting and merging.
Commit 1: Fixes #116, scrub types used for indexing and identification
Commit 2: Fixes #117, change local array refs to object pointer
Commit 3: Fixes #118, implement range checking functions
Testing performed
Build and run all tests (Pending run with BVT setup)
Expected behavior changes
Cleaner codebase with more consistent error checking (or at least one step toward that)
System(s) tested on
Debian
Contributor Info - All information REQUIRED for consideration of pull request
Joseph Hickey, Vantage Systems, Inc.